Skip to content

feat: pluggable JSON schema validator providers #1012

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 10 commits into from
Oct 25, 2025
Merged

feat: pluggable JSON schema validator providers #1012

merged 10 commits into from
Oct 25, 2025
+2,373 −134

Conversation

@mattzcarey
Copy link
Contributor

@mattzcarey mattzcarey commented Oct 7, 2025 •
edited
Loading

Fixes #689 by adding the concept of pluggable JSON schema validators.

Supports ajv and cfworker (works well on edge runtimes).
Also upgrades to ajv v8

Motivation and Context

AJV consistently causes issues in Workers due to its use of eval. You can see a repro of this using the latest v8 version and other related bugs here

How Has This Been Tested?

This is tested against a non exhaustive set of json schema 2020-12 spec (in prep for SEP-1330) and the limited set of features supported here

Breaking Changes

This is not breaking and should not be noticed by users unless it unblocks them on edge runtimes.
Users can opt in by declaring a jsonSchemaValidator in both Client and Server

{                
   jsonSchemaValidator: ajvValidator // or cfWorkerValidator
}

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@mattzcarey mattzcarey requested a review from a team as a code owner October 7, 2025 10:52
@mattzcarey mattzcarey changed the title Elicitation broken in Cloudflare Worker fix: elicitation json schema validation in Cloudflare Worker Oct 7, 2025
@paoloricciuti
Copy link

Elicitation can't work on cloudflare workers at all because it requires a second request from the client to resolve the first request and that's impossible in cloudflare architecture

@mattzcarey
Copy link
Contributor Author

mattzcarey commented Oct 7, 2025 •
edited
Loading

Elicitation can't work on cloudflare workers at all because it requires a second request from the client to resolve the first request and that's impossible in cloudflare architecture

Hey, I'm working on this at Cloudflare at the moment. We use a Durable Object to keep the state during the elicitation (and for all stateful MCP interactions). Multiple requests from the client can hit the same Durable Object :)

You can read more about it here

@paoloricciuti
Copy link

@mattzcarey i used the same approach but was still stopped by cloudflare...would you be available for a quick chat some of those days?

@mattzcarey
Copy link
Contributor Author

@mattzcarey i used the same approach but was still stopped by cloudflare...would you be available for a quick chat some of those days?

Sure. I'm still very new (I started yesterday) but it's my job to get this working.

@paoloricciuti
Copy link

@mattzcarey do you have discord or where can I contact you?

@mattzcarey
Copy link
Contributor Author

@mattzcarey do you have discord or where can I contact you?

I'm in the cloudflare discord :)

@jake-danton
Copy link

I don't think it is the correct move to fully switch over to @cfworker/json-schema for everyone as it is significantly slower than Ajv. For validating the Tool schema in this library, it runs 40x - 80x slower and takes much longer to generate the validator as well. So while it unlocks using features like elicitInput in this library for those of us who use Cloudflare or other runtimes without eval, it will be a performance regression for most users.

I am pushing for the library to move away from these hard-coded dependencies (Ajv, Zod, etc) to a flexible core with the ability to inject them, allowing developers to choose what is best for their use case. I created a fork @enth/mcp-sdk that allows you to use the any JSON schema validator (including pre-compiled ones) or schema definition library. Would love to hear your thoughts on it and if that approach would work for your use case.

@mattzcarey
Copy link
Contributor Author

mattzcarey commented Oct 8, 2025 •
edited
Loading

Hey @jake-danton awesome work on the fork. I ran some benchmarks on this change with some simple examples. I can imagine with deeply nested schemas this gap would widen.

Schema Type     | AJV (M ops/sec) | CFWorker (M ops/sec) | Performance Ratio
----------------------------------------------------------------------------
String Email    | 14.6M           | 6.0M                 | 2.4x
String URI      | 12.0M           | 8.2M                 | 1.5x
String Date     | 11.6M           | 8.4M                 | 1.4x
String DateTime | 4.3M            | 3.8M                 | 1.1x
Number          | 34.9M           | 19.5M                | 1.8x
Integer         | 34.7M           | 19.3M                | 1.8x

Slower sure, but I'm not sure this constitutes a completely new dev experience. There are other efficiencies that are lower hanging fruit in the SDK nevermind the fact that an LLM is always going to be the slowest part of this system.

There is ongoing chat about this internally, we could support AJV in its current form but the validator would have to be declared in the global scope for the eval to work. Maybe also an option.

I'll check out your repo and benchmarks more tomorrow, it is definitely cool work.

@felixweinberger felixweinberger mentioned this pull request Oct 13, 2025
Closed
@felixweinberger
Copy link
Contributor

I believe this might resolve: #857

@felixweinberger felixweinberger linked an issue Oct 13, 2025 that may be closed by this pull request
[Sampling/Elicitation] Stateless servers silently failing on session-requiring features #857
Closed
@felixweinberger felixweinberger mentioned this pull request Oct 13, 2025
Closed
@felixweinberger felixweinberger linked an issue Oct 13, 2025 that may be closed by this pull request
Error compiling schema. #923
Closed
@dsp-ant
Copy link
Member

dsp-ant commented Oct 14, 2025

I think the right approach here is to make the validator configurable and less of a hard-dependency as @jake-danton suggests. I am wary about moving to different validator given perf implications and since ajv seems to be the de-facto standard in the ecosystem. We must also consider that any validator must be able to support 2020-12 as a schema, since we are moving to this as the default schema.

@mattzcarey
Copy link
Contributor Author

I think the right approach here is to make the validator configurable and less of a hard-dependency as @jake-danton suggests. I am wary about moving to different validator given perf implications and since ajv seems to be the de-facto standard in the ecosystem. We must also consider that any validator must be able to support 2020-12 as a schema, since we are moving to this as the default schema.

Thanks @dsp-ant will make a pr for this direction.

@mattzcarey mattzcarey force-pushed the fix/elliciation-broken-in-edge-worker branch from 5dc06e3 to 05f23aa Compare October 17, 2025 15:33
@mattzcarey mattzcarey changed the title fix: elicitation json schema validation in Cloudflare Worker feat: pluggable JSON schema validator providers Oct 17, 2025
@ihrpr ihrpr added this to the HPR milestone Oct 23, 2025
bhosmer-ant
bhosmer-ant requested changes Oct 24, 2025
View reviewed changes
Copy link
Contributor

@bhosmer-ant bhosmer-ant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mattzcarey - thanks for working on this!

I believe there's an issue with removing ajv from dependencies though, since the SDK still needs it available for the default validator. (Adding it to devDependencies makes it available when working on the SDK itself, but e.g. apps that depend on the SDK won't bring it in.)

IIUC restoring the hard dependency should be fine w.r.t. the original problem, since the ajv's compile() will still never run when a custom validator is passed in.

fwiw here's what I did to confirm the problem:

  1. Build and pack the SDK:
    npm install
    npm run build
    npm pack # Creates modelcontextprotocol-sdk-1.20.1.tgz

  2. Create test dir and install sdk (doesn't bring in peer or dev deps):
    mkdir /tmp/mcp-test
    cd /tmp/mcp-test
    npm init -y
    npm install /path/to/modelcontextprotocol-sdk-1.20.1.tgz

  3. try to import:
    echo "const { Client } = require('@modelcontextprotocol/sdk/client')" > test.js
    node test.js

node:internal/modules/cjs/loader:1413
  throw err;
  ^

Error: Cannot find module 'ajv'
Require stack:
- /private/tmp/mcp-test/node_modules/@modelcontextprotocol/sdk/dist/cjs/validation/ajv-provider.js
- /private/tmp/mcp-test/node_modules/@modelcontextprotocol/sdk/dist/cjs/client/index.js
- /private/tmp/mcp-test/test.js
...

@mattzcarey mattzcarey force-pushed the fix/elliciation-broken-in-edge-worker branch 2 times, most recently from 6dfea9f to ca77e8b Compare October 24, 2025 09:39
@mattzcarey mattzcarey force-pushed the fix/elliciation-broken-in-edge-worker branch from ca77e8b to 1e75e04 Compare October 24, 2025 09:44
@mattzcarey
Copy link
Contributor Author

Great spot thanks @bhosmer-ant I added ajv back as a dependancy :)

@felixweinberger felixweinberger added enhancement New feature or request needs maintainer action Potentially serious issue - needs proactive fix and maintainer attention labels Oct 24, 2025
bhosmer-ant
bhosmer-ant approved these changes Oct 24, 2025
View reviewed changes
Copy link
Contributor

@bhosmer-ant bhosmer-ant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! LGTM

@bhosmer-ant bhosmer-ant merged commit 5a8fb39 into modelcontextprotocol:main Oct 25, 2025
2 checks passed
@mattzcarey mattzcarey deleted the fix/elliciation-broken-in-edge-worker branch October 25, 2025 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bhosmer-ant bhosmer-ant bhosmer-ant approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request needs maintainer action Potentially serious issue - needs proactive fix and maintainer attention

Projects

None yet

Milestone

HPR

7 participants

@mattzcarey @paoloricciuti @jake-danton @felixweinberger @dsp-ant @bhosmer-ant @ihrpr